-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Implement the internal feature cfg_target_has_reliable_f16_f128
#140323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
98cf08a
to
e7fb9c5
Compare
@bors try |
Implement the internal feature `cfg_target_has_reliable_f16_f128` Support for `f16` and `f128` is varied across targets, backends, and backend versions. Eventually we would like to reach a point where all backends support these approximately equally, but until then we have to work around some of these nuances of support being observable. Introduce the `target_has_reliable_f16_f128` configuration option, gated behind `cfg_target_has_reliable_f16_f128`, as an indicator for whether or not the backend supports these types to a point that they can and should be tested. The configuration mostly follows the logic used by `target_feature`, and similarly takes a value: #[cfg(target_has_reliable_f16_f128 = "f16")] Accepted values are: * `f16` * `f16-math` * `f128` * `f128-math` `f16` and `f128` indicate that basic arithmetic for the type works correctly. The `-math` versions indicate that anything relying on `libm` works correctly, since sometimes this hits a separate class of codegen bugs. These options match configuration set by the build script at [1]. The logic for LLVM support is duplicated as-is from the same script. There are a few possible updates that will come as a follow up. The config introduced here is not intended to ever become stable, it is only intended to replace the build scripts for `std` tests and in `compiler-builtins` that don't have any way to set configuration based on the codegen backend. MCP: rust-lang/compiler-team#866 Closes: rust-lang/compiler-team#866 [1]: https://github.com/rust-lang/rust/blob/555e1d0386f024a8359645c3217f4b3eae9be042/library/std/build.rs#L84-L186 --- The second commit makes use of this config to replace `cfg_{f16,f128}{,_math}` in `library/`. try-job: aarch64-gnu try-job: i686-msvc-1 try-job: test-various try-job: x86_64-gnu try-job: x86_64-msvc-ext2
e7fb9c5
to
4d7bb41
Compare
Some changes occurred in src/doc/rustc/src/check-cfg.md cc @Urgau |
Hm... I think I'm actually leaning toward four separate key-only Will change it over tomorrow unless anyone has a stronger preference. @rustbot author |
Reminder, once the PR becomes ready for a review, use |
b1025c9
to
5abd112
Compare
Changed so this is split across four different @rustbot review |
ac61743
to
304739d
Compare
Since you have already started taking a look, r? @Urgau |
This comment was marked as outdated.
This comment was marked as outdated.
/// # #![cfg_attr(not(bootstrap), feature(cfg_target_has_reliable_f16_f128))] | ||
/// # #![cfg_attr(not(bootstrap), expect(internal_features))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To bad that rustdoc
#![doc(test(attr(...)))]
attribute only works at the crate-root level (and not module level), otherwise we could have saved us a lot of redundancy by just having in each module:
#![doc(test(attr(feature(cfg_target_has_reliable_f16_f128))))]
#![doc(test(attr(expect(internal_features))))]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that would be so nice. Agreed, tests are too noisy with config, but at least they shouldn't need to be updated very frequently. (and bootstrap will go away soon)
Support for `f16` and `f128` is varied across targets, backends, and backend versions. Eventually we would like to reach a point where all backends support these approximately equally, but until then we have to work around some of these nuances of support being observable. Introduce the `cfg_target_has_reliable_f16_f128` internal feature, which provides the following new configuration gates: * `cfg(target_has_reliable_f16)` * `cfg(target_has_reliable_f16_math)` * `cfg(target_has_reliable_f128)` * `cfg(target_has_reliable_f128_math)` `reliable_f16` and `reliable_f128` indicate that basic arithmetic for the type works correctly. The `_math` versions indicate that anything relying on `libm` works correctly, since sometimes this hits a separate class of codegen bugs. These options match configuration set by the build script at [1]. The logic for LLVM support is duplicated as-is from the same script. There are a few possible updates that will come as a follow up. The config introduced here is not planned to ever become stable, it is only intended to replace the build scripts for `std` tests and `compiler-builtins` that don't have any way to configure based on the codegen backend. MCP: rust-lang/compiler-team#866 Closes: rust-lang/compiler-team#866 [1]: https://github.com/rust-lang/rust/blob/555e1d0386f024a8359645c3217f4b3eae9be042/library/std/build.rs#L84-L186
304739d
to
f32222b
Compare
New compiler configuration has been introduced that is designed to replace the build script configuration `reliable_f16`, `reliable_f128`, `reliable_f16_math`, and `reliable_f128_math`. Do this replacement here, which allows us to clean up `std`'s build script. All tests are gated by `#[cfg(bootstrap)]` rather than doing a more complicated `cfg(bootstrap)` / `cfg(not(bootstrap))` split since the next beta split is within two weeks.
f32222b
to
dfa972e
Compare
The doctests changes are still pretty horrendous, but without better rustdoc support and cfg aliases I don't see how we can make that better. r=me if the try run is okay |
Implement the internal feature `cfg_target_has_reliable_f16_f128` Support for `f16` and `f128` is varied across targets, backends, and backend versions. Eventually we would like to reach a point where all backends support these approximately equally, but until then we have to work around some of these nuances of support being observable. Introduce the `cfg_target_has_reliable_f16_f128` internal feature, which provides the following new configuration gates: * `cfg(target_has_reliable_f16)` * `cfg(target_has_reliable_f16_math)` * `cfg(target_has_reliable_f128)` * `cfg(target_has_reliable_f128_math)` `reliable_f16` and `reliable_f128` indicate that basic arithmetic for the type works correctly. The `_math` versions indicate that anything relying on `libm` works correctly, since sometimes this hits a separate class of codegen bugs. These options match configuration set by the build script at [1]. The logic for LLVM support is duplicated as-is from the same script. There are a few possible updates that will come as a follow up. The config introduced here is not planned to ever become stable, it is only intended to replace the build scripts for `std` tests and `compiler-builtins` that don't have any way to configure based on the codegen backend. MCP: rust-lang/compiler-team#866 Closes: rust-lang/compiler-team#866 [1]: https://github.com/rust-lang/rust/blob/555e1d0386f024a8359645c3217f4b3eae9be042/library/std/build.rs#L84-L186 --- The second commit makes use of this config to replace `cfg_{f16,f128}{,_math}` in `library/`. I omitted providing a `cfg(bootstrap)` configuration to keep things simpler since the next beta branch is in two weeks. try-job: aarch64-gnu try-job: i686-msvc-1 try-job: test-various try-job: x86_64-gnu try-job: x86_64-msvc-ext2
⌛ Trying commit dfa972e with merge 815cc5475dfec1f5cb06222ac0472b2e9e4d4e3e... |
Support for
f16
andf128
is varied across targets, backends, and backend versions. Eventually we would like to reach a point where all backends support these approximately equally, but until then we have to work around some of these nuances of support being observable.Introduce the
cfg_target_has_reliable_f16_f128
internal feature, which provides the following new configuration gates:cfg(target_has_reliable_f16)
cfg(target_has_reliable_f16_math)
cfg(target_has_reliable_f128)
cfg(target_has_reliable_f128_math)
reliable_f16
andreliable_f128
indicate that basic arithmetic for the type works correctly. The_math
versions indicate that anything relying onlibm
works correctly, since sometimes this hits a separate class of codegen bugs.These options match configuration set by the build script at 1. The logic for LLVM support is duplicated as-is from the same script. There are a few possible updates that will come as a follow up.
The config introduced here is not planned to ever become stable, it is only intended to replace the build scripts for
std
tests andcompiler-builtins
that don't have any way to configure based on the codegen backend.MCP: rust-lang/compiler-team#866
Closes: rust-lang/compiler-team#866
The second commit makes use of this config to replace
cfg_{f16,f128}{,_math}
inlibrary/
. I omitted providing acfg(bootstrap)
configuration to keep things simpler since the next beta branch is in two weeks.try-job: aarch64-gnu
try-job: i686-msvc-1
try-job: test-various
try-job: x86_64-gnu
try-job: x86_64-msvc-ext2
r? @ghost